feat(federation): add room invites support #37532
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughAdds federated invitation handling: new POST /v1/rooms.invite endpoint invoking FederationMatrix.handleInvite, INVITED subscription status with inviter metadata, accept/reject flows and helpers, RoomService subscription helpers, message types/locales for invites, Matrix leave/send_leave routes, model/typing updates, and updated tests/helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as REST API (/v1/rooms.invite)
participant FedMatrix as FederationMatrix
participant Subs as Subscriptions Model
participant RoomSvc as RoomService
participant FedSDK as Federation SDK
Client->>API: POST /v1/rooms.invite { roomId, action }
API->>FedMatrix: handleInvite(roomId, currentUserId, action)
FedMatrix->>Subs: findInvitedSubscription(roomId, userId)
Subs-->>FedMatrix: subscription (status: INVITED) / null
alt subscription exists and action == "accept"
FedMatrix->>FedSDK: process/accept invite
FedSDK-->>FedMatrix: result
FedMatrix->>RoomSvc: performAcceptRoomInvite(room, subscription, user)
RoomSvc->>Subs: acceptInvitationById(subscriptionId)
RoomSvc->>RoomSvc: run hooks, save system message, post-join callbacks
else subscription exists and action == "reject"
FedMatrix->>FedSDK: reject invite
FedSDK-->>FedMatrix: result
FedMatrix->>Subs: optional cleanup/notify
else no invited subscription
FedMatrix-->>API: failure (400/403)
end
FedMatrix-->>API: success/failure
API-->>Client: 200 / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/packages/federation-matrix/src/api/_matrix/profiles.ts (1)
368-386: Inconsistent null handling between the two response paths needs fixing.The optional chaining added on line 376 (
response?.[field]) indicatesresponsemay be undefined. However, line 383 returnsresponsedirectly without the same safety check, creating an inconsistency.Evidence from the codebase shows similar SDK methods can return null/undefined:
downloadFromRemoteServerexplicitly checks for null (MatrixMediaService.ts:107)getEventByIdchecks if undefined before returning (transactions.ts:443)If
queryProfilecan return undefined:
- Both code paths (line 376 and 383) need consistent null handling
- Returning HTTP 200 with undefined body may violate the schema contract
Recommendation: Add explicit error handling if response is null/undefined, or document that
queryProfileguarantees a valid object to justify removing the optional chaining.
🧹 Nitpick comments (8)
packages/i18n/src/locales/en.i18n.json (1)
5565-5566: Copy tweak for article consistency.Use “the” as in the previous line (“to the room”).
- "User_rejected_invitation_to_room": "rejected invitation to room", + "User_rejected_invitation_to_room": "rejected the invitation to the room",apps/meteor/app/api/server/v1/rooms.ts (1)
1-1: rooms.invite route wiring looks good; minor nits on validation/error handlingThe new
rooms.inviteroute correctly restricts actions to the authenticated user (viathis.userIdinFederationMatrix.handleInvite) and validates the payload viaisRoomsInviteProps, which is great for safety.Two small suggestions:
- Since
isRoomsInvitePropsalready constrainssubscriptionIdandaction, the explicit!subscriptionIdandaction !== 'accept' && action !== 'reject'checks are redundant; you can rely purely on ajv to avoid duplicate validation logic.- In the
catchblock, you currently only surface the error message to the client; consider also logging the raw error server‑side (if not already logged inhandleInvite) to aid debugging of Matrix/federation issues.Also applies to: 18-19, 1079-1104
packages/core-services/src/types/IRoomService.ts (1)
34-48: addUserToRoom options extension matches subscription invite metadataAdding
invited?: booleanandfederation?: { inviteEventId?: string; inviterUsername?: string }to theaddUserToRoomoptions cleanly exposes the invite metadata used by the new Matrix flows and matches theISubscriptionshape.If you expect this metadata to evolve, you might consider typing
federationas aPick<ISubscription, 'federation'>['federation']to avoid duplicating the structure, but the current explicit interface is also fine.ee/packages/federation-matrix/src/FederationMatrix.ts (1)
1-1: handleInvite implementation is consistent; consider centralizing Matrix user ID derivationThe new
handleInvitemethod correctly:
- Ensures the subscription belongs to the current user and is flagged
invited: true.- Requires
federation.inviteEventIdto be present.- Looks up the user and emits appropriate system messages for accept (
uj) and reject (uir).A couple of small improvements:
- You compute
matrixUserIdbefore checkinguser.username; for readability, consider moving the!user.usernameguard before constructing the local MXID branch.- The logic that turns a
IUserinto a Matrix user ID now exists in several places (here and in helpers). Extracting a shared helper for "user → Matrix ID" would reduce drift if the federation rules change.Also applies to: 10-10, 902-937
ee/packages/federation-matrix/src/events/helpers.ts (1)
12-84: Federated user/room helpers look correct; minor cleanups possibleThe helpers nicely encapsulate the recurring patterns:
getOrCreateFederatedUsercorrectly distinguishes local vs remote Matrix IDs viagetUsernameServername, avoids auto‑creating local users, and delegates remote creation tocreateOrUpdateFederatedUser.getOrCreateFederatedRoomuses the Matrix room ID as the lookup key (federation.mrid) and passesfederatedRoomId/fnameintoRoom.create, which is consistent with the newICreateRoomOptionsfields.Small nits:
- The
_inviterMatrixIdparameter is currently unused; if there’s no near‑term use planned, you can drop it to keep the signature minimal.- For clarity, it might be worth renaming the first parameter of
getOrCreateFederatedRoomto something likematrixRoomIdto distinguish it from the human display name.apps/meteor/app/lib/server/functions/addUserToRoom.ts (1)
22-41: Invite/federation plumbing in addUserToRoom is coherent; tiny message nitThe changes here line up well with the new invite flow:
invitedandfederationoptions are threaded through and conditionally added to the subscription document, matching the newISubscriptionfields.- The system-message logic now:
- Requires
inviter.usernamebefore using inviter‑specific branches.- Emits
uiwheninvitedis true, carrying the inviter asuin the payload, which matches the new invite message type.Minor nit: the
uibranch doesn’t reuse theextraDataobject or set an explicittslike theau/team messages. If you want identical timestamp semantics, you could pass{ ts: now, u: { ... } }instead of constructing a separate object.Also applies to: 102-113, 123-147
ee/packages/federation-matrix/src/events/member.ts (2)
55-111: handleInvite flow is solid; tighten room name fallback and type usageThe invite handler correctly:
- Resolves/creates inviter and invitee via
getOrCreateFederatedUser.- Derives room type (
'd'for direct,'c'otherwise) and a human‑readableroomFName.- Ensures a federated room exists via
getOrCreateFederatedRoom, then callsRoom.addUserToRoomwithinvitedandfederationmetadata so downstream logic can treat it as a pending invite.Two small points to consider:
- The fallback
roomName = \${room_id}:${roomOriginDomain}`will produce strings like!abc:server:undefinedwhenroomOriginDomainis missing; you might want to default to justroom_idin that case to avoid oddfname` values._inviterMatrixIdpassed intogetOrCreateFederatedRoomis unused; either remove the parameter or start using it (e.g., if you later need the Matrix ID inside room creation).
113-158: Join handler correctly distinguishes direct joins vs invite acceptanceThe join handler behavior looks good:
- If no local room exists yet, it falls back to
membershipJoinAction, preserving the legacy behavior.- If there’s no subscription for the joining user, it also delegates to
membershipJoinAction, treating it as a regular join.- When a subscription exists, it interprets the event as “accepting an invite” and unsets
invitedand the invite metadata while reopening the subscription.One minor improvement: you might want to guard the “accept invite” path with
if (subscription.invited)and simply return wheninvitedis falsy, to avoid unnecessary$unset/$setupdates on repeated join events for already‑joined users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
apps/meteor/app/api/server/v1/rooms.ts(3 hunks)apps/meteor/app/lib/lib/MessageTypes.ts(1 hunks)apps/meteor/app/lib/server/functions/addUserToRoom.ts(4 hunks)apps/meteor/lib/publishFields.ts(1 hunks)apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts(1 hunks)apps/meteor/server/modules/watchers/watchers.module.ts(1 hunks)ee/packages/federation-matrix/src/FederationMatrix.ts(3 hunks)ee/packages/federation-matrix/src/api/_matrix/invite.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/profiles.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/rooms.ts(1 hunks)ee/packages/federation-matrix/src/events/helpers.ts(1 hunks)ee/packages/federation-matrix/src/events/member.ts(2 hunks)packages/apps-engine/src/definition/messages/MessageType.ts(1 hunks)packages/core-services/src/types/IFederationMatrixService.ts(2 hunks)packages/core-services/src/types/IRoomService.ts(1 hunks)packages/core-typings/src/IMessage/IMessage.ts(1 hunks)packages/core-typings/src/ISubscription.ts(2 hunks)packages/i18n/src/locales/en.i18n.json(2 hunks)packages/message-types/src/registrations/common.ts(1 hunks)packages/rest-typings/src/v1/rooms.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/lib/dataExport/exportRoomMessagesToFile.tspackages/core-services/src/types/IRoomService.tsee/packages/federation-matrix/src/api/_matrix/rooms.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/helpers.tsee/packages/federation-matrix/src/api/_matrix/invite.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/lib/publishFields.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/api/server/v1/rooms.tsee/packages/federation-matrix/src/api/_matrix/rooms.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/api/server/v1/rooms.tspackages/core-services/src/types/IRoomService.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/api/server/v1/rooms.tspackages/core-services/src/types/IRoomService.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/lib/publishFields.tspackages/core-typings/src/ISubscription.tsapps/meteor/server/modules/watchers/watchers.module.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/app/lib/server/functions/addUserToRoom.ts
🧬 Code graph analysis (6)
ee/packages/federation-matrix/src/FederationMatrix.ts (2)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(10-81)packages/core-typings/src/IUser.ts (2)
IUser(186-255)isUserNativeFederated(276-277)
ee/packages/federation-matrix/src/events/member.ts (3)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
handleInvite(902-937)ee/packages/federation-matrix/src/events/helpers.ts (2)
getOrCreateFederatedUser(12-48)getOrCreateFederatedRoom(50-84)packages/core-typings/src/IRoom.ts (1)
RoomID(12-12)
ee/packages/federation-matrix/src/events/helpers.ts (4)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)ee/packages/federation-matrix/src/FederationMatrix.ts (3)
getUsernameServername(73-85)createOrUpdateFederatedUser(92-133)extractDomainFromMatrixUserId(60-66)packages/core-typings/src/IRoom.ts (2)
RoomID(12-12)IRoom(21-95)packages/core-services/src/index.ts (1)
Room(169-169)
apps/meteor/app/lib/server/functions/addUserToRoom.ts (1)
packages/core-services/src/index.ts (1)
Message(184-184)
packages/core-services/src/types/IFederationMatrixService.ts (2)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(10-81)packages/core-typings/src/IUser.ts (1)
IUser(186-255)
apps/meteor/app/api/server/v1/rooms.ts (2)
packages/rest-typings/src/v1/rooms.ts (1)
isRoomsInviteProps(712-712)ee/packages/federation-matrix/src/FederationMatrix.ts (1)
FederationMatrix(137-938)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (17)
ee/packages/federation-matrix/src/api/_matrix/rooms.ts (1)
189-189: LGTM! Good defensive programming.The optional chaining prevents potential runtime errors when room names are undefined during filtering.
apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts (1)
82-89: LGTM! Consistent implementation.The new message type handlers follow the established pattern and properly apply username obfuscation for the 'ui' case, consistent with similar user action types like 'au'.
apps/meteor/lib/publishFields.ts (1)
45-46: LGTM! Subscription field additions.The new
invitedandfederationfields extend subscription projections to support the federation invite workflow.packages/core-services/src/types/IFederationMatrixService.ts (2)
1-1: LGTM! Required import for new method.Adding
ISubscriptionimport supports the newhandleInvitemethod signature.
31-31: LGTM! Well-typed method signature.The
handleInvitemethod signature is clear and properly typed with union type for the action parameter.packages/core-typings/src/IMessage/IMessage.ts (1)
71-72: LGTM! Type definitions for new message types.The new 'ui' and 'uir' message type literals are properly added to the MessageTypes array, enabling type-safe usage across the codebase.
packages/apps-engine/src/definition/messages/MessageType.ts (1)
78-81: LGTM! Clear type definitions with descriptive comments.The new message types are well-documented and properly integrated into the MessageType union.
packages/message-types/src/registrations/common.ts (1)
28-38: LGTM! Consistent system message registrations.The new 'ui' and 'uir' message type registrations follow established patterns and properly integrate with the i18n system.
ee/packages/federation-matrix/src/api/_matrix/invite.ts (3)
1-10: LGTM!The import changes are clean and all imported symbols are properly utilized in the code.
174-271: LGTM!The route handler correctly validates parameters, checks permissions, and returns appropriate HTTP status codes. The permission check at line 220 appropriately returns a 403 response to inform the remote federation server that the user cannot access federation, which is the correct behavior for an HTTP endpoint.
136-172: TheacceptInvitefunction is actively used in the codebase and is not dead code.Verification confirms that
acceptInviteis imported and called inee/packages/federation-matrix/src/FederationMatrix.tsat line 570 within an invite handling promise chain. The original review comment's concern was based on incomplete information.Likely an incorrect or invalid review comment.
packages/rest-typings/src/v1/rooms.ts (2)
691-712: LGTM!The type definition and schema are well-structured with proper validation. The schema correctly enforces required fields, constrains the action to valid values, and prevents additional properties.
895-897: LGTM!The endpoint definition is consistent with the rest of the API and correctly references the new
RoomsInvitePropstype.packages/core-typings/src/ISubscription.ts (1)
76-80: LGTM!The new fields are well-structured to support federation invite tracking. The
invited?: truepattern effectively creates a boolean flag that defaults to undefined, and thefederationobject provides appropriate metadata for federated invites.packages/i18n/src/locales/en.i18n.json (1)
3366-3367: Message type labels properly wired; no action needed.The new types
uianduirare correctly defined in MessageTypes.ts (lines 33–39) with matching i18n labels in the JSON file. The Hide_System_Messages setting usesMessageTypesValuesdynamically, automatically including all registered message types. Verification confirms proper wiring across the system.apps/meteor/server/modules/watchers/watchers.module.ts (1)
140-147: Including invited/federation in watched subscription fields is appropriateExtending the
Pick<ISubscription, ...>to includeinvitedandfederationkeeps the watcher’s projection in sync withsubscriptionFieldsand ensures invite state is broadcast to other instances/clients.ee/packages/federation-matrix/src/events/helpers.ts (1)
86-103: extractDomainFromId / extractUsernameFromMatrixId are safe, defensive wrappersBoth extraction helpers handle malformed IDs by logging and returning a fallback, which is appropriate for event‑driven federation code where robustness beats hard failures.
No issues here; just ensure call sites are prepared for the empty‑string / original‑ID fallbacks when verification is important.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/app/lib/server/functions/removeUserFromRoom.ts (1)
35-37: Critical:invitedfield missing from projection.The subscription is fetched with only
{ _id: 1 }in the projection, but line 48 checkssubscription.invited. Sinceinvitedis not included in the projection, it will always beundefined, causing the new conditional branch (lines 48-49) to never execute.Apply this diff to include the
invitedfield:const subscription = await Subscriptions.findOneByRoomIdAndUserId(rid, user._id, { - projection: { _id: 1 }, + projection: { _id: 1, invited: 1 }, });
🧹 Nitpick comments (1)
ee/packages/federation-matrix/src/events/member.ts (1)
71-71: Remove redundant membership check.Line 71 checks
content.membership === 'invite', but this function is only called when membership is already 'invite' (line 184 in the dispatcher). This condition is always true here.Apply this diff:
-const roomType = content.membership === 'invite' && content?.is_direct ? 'd' : 'c'; +const roomType = content?.is_direct ? 'd' : 'c';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/meteor/app/lib/lib/MessageTypes.ts(1 hunks)apps/meteor/app/lib/server/functions/removeUserFromRoom.ts(1 hunks)ee/packages/federation-matrix/src/FederationMatrix.ts(2 hunks)ee/packages/federation-matrix/src/events/member.ts(2 hunks)packages/core-typings/src/ISubscription.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core-typings/src/ISubscription.ts
- apps/meteor/app/lib/lib/MessageTypes.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/app/lib/server/functions/removeUserFromRoom.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
🧬 Code graph analysis (2)
ee/packages/federation-matrix/src/FederationMatrix.ts (2)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(10-81)packages/core-typings/src/IUser.ts (2)
IUser(186-255)isUserNativeFederated(276-277)
ee/packages/federation-matrix/src/events/member.ts (2)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
handleInvite(902-935)ee/packages/federation-matrix/src/events/helpers.ts (2)
getOrCreateFederatedUser(12-48)getOrCreateFederatedRoom(50-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
apps/meteor/app/lib/server/functions/removeUserFromRoom.ts (1)
48-49: Verify the semantic correctness of message type 'uir'—this is a confirmed issue.The 'uir' message type is defined as "user rejected invitation to room" (per the comment in
apps/meteor/app/lib/lib/MessageTypes.ts). However, the code at lines 48-49 sends this message when an admin/moderator removes a user in an invited state (indicated byoptions.byUserbeing present), not when the user themselves rejects the invitation.The commit history shows that 'uir' was previously used in federation code where a user actively rejected an invite. Moving this to the common
removeUserFromRoomfunction conflates two different scenarios:
- User-initiated rejection (federation workflow)
- Admin-initiated removal of invited user (your new code path)
The translation displays "User rejected invitation to room," which suggests the user took the action, creating confusion in user-facing messages when an admin revokes an invite.
Recommended actions:
- Define a distinct message type for admin invite revocation (e.g., 'uir-revoked', 'air', or 'invite-revoked'), or
- If intentionally reusing 'uir' for both scenarios, update the message type definition and translation to reflect both user rejection and admin revocation.
ee/packages/federation-matrix/src/events/member.ts (3)
113-158: LGTM! Proper invite acceptance flow.The join handler correctly distinguishes between direct joins (no prior invite) and invite acceptances, with appropriate subscription state updates to clear invite metadata and activate the subscription.
160-177: LGTM! Clean leave implementation.The leave handler appropriately delegates to the
Room.removeUserFromRoomservice method, which handles all necessary cleanup including subscription removal and user count adjustments.
182-197: LGTM! Clear event dispatching.The switch-based dispatcher cleanly routes membership events to their respective handlers with appropriate logging for unknown membership types.
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
929-934: Add explicit handling for invalid action values.The method uses separate
ifstatements for 'accept' and 'reject' actions. If an invalid action value is passed, the method silently does nothing, which could mask bugs.Apply this diff to use if/else and explicitly handle invalid actions:
if (action === 'accept') { await federationSDK.acceptInvite(subscription.federation?.inviteEventId, matrixUserId); -} -if (action === 'reject') { +} else if (action === 'reject') { await federationSDK.rejectInvite(subscription.federation?.inviteEventId, matrixUserId); +} else { + throw new Error(`Invalid action: ${action}`); }⛔ Skipped due to learnings
Learnt from: ricardogarim Repo: RocketChat/Rocket.Chat PR: 37377 File: apps/meteor/ee/server/hooks/federation/index.ts:86-88 Timestamp: 2025-11-04T16:49:19.107Z Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
ricardogarim
force-pushed
the
feat/invites
branch
4 times, most recently
from
4776e08 to
3e950a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
ee/packages/federation-matrix/src/events/member.ts (1)
17-21: Validate inviter username before usage.The code uses
inviterUser.usernameon lines 62 and 71 without validation. SinceIUser.usernameis optional, this could lead to:
- Line 62: Unsafe cast to
UserIDif username is undefined- Line 71: Storing
undefinedasinviterUsernamein federation metadataAdd validation after line 21:
const inviterUser = await getOrCreateFederatedUser(senderId as UserID); if (!inviterUser) { logger.error(`Failed to get or create inviter user: ${senderId}`); return; } +if (!inviterUser.username) { + logger.error(`Inviter user ${inviterUser._id} has no username`); + return; +}Note: This issue was flagged in a previous review but appears unresolved.
Also applies to: 57-63, 69-72
🧹 Nitpick comments (4)
apps/meteor/app/lib/server/functions/acceptRoomInvite.ts (1)
9-20: Guard against mismatched room/subscription/user tripletsThe function assumes
room,subscription, anduserare consistent (same room and user). To make this helper safer against accidental misuse by future callers, consider asserting:
subscription.rid === room._idsubscription.u._id === user._idbefore proceeding, and throwing a
Meteor.Errorif they do not match. This keeps the trust boundary clear while reusing the helper in other call sites.apps/meteor/app/lib/server/methods/addUsersToRoom.ts (1)
110-121: MakeinviteResult[0]handling robust against empty resultsThe new federation invite path is wired correctly, but
inviteResult[0]assumes thatinviteUsersToRoomalways returns at least one element:const inviteResult = await FederationMatrix.inviteUsersToRoom(...); inviteOptions = { invited: true, federation: { inviteEventId: inviteResult[0].event_id, inviterUsername: user.username, }, };Today this holds when called from local user flows, but
inviteUsersToRoomcan return an empty array if the caller ever passes a combination that leads tousersToInvite.length === 0. To future‑proof this helper, consider guarding the assignment:- const inviteResult = await FederationMatrix.inviteUsersToRoom(room, [newUser.username], user); - inviteOptions = { - invited: true, - federation: { - inviteEventId: inviteResult[0].event_id, - inviterUsername: user.username, - }, - }; + const inviteResult = await FederationMatrix.inviteUsersToRoom(room, [newUser.username], user); + if (inviteResult.length > 0) { + inviteOptions = { + invited: true, + federation: { + inviteEventId: inviteResult[0].event_id, + inviterUsername: user.username, + }, + }; + }This keeps the invite metadata optional but avoids potential
undefinedaccess if the federation layer changes behavior later.Since this depends on
FederationMatrix.inviteUsersToRoom’s contract, please confirm its “no‑op” behavior and whether it can ever legitimately return[]for this call pattern.Also applies to: 123-124
apps/meteor/app/lib/server/functions/addUserToRoom.ts (1)
134-137: Consider tightening inviter type to require username.The new UI message path on line 134 uses
inviter.usernameon line 136 without validation. While this follows the existing pattern (line 129), a past reviewer (ggazzo) suggested "it should be impossible to have an inviter without username, what about fix the type?"Consider updating the
inviterparameter type fromPick<IUser, '_id' | 'username'>toPick<IUser, '_id'> & Required<Pick<IUser, 'username'>>to enforce username at the type level.Note: This applies to the entire function, not just the new code.
ee/packages/federation-matrix/src/events/helpers.ts (1)
86-93: Consider returning null instead of empty string on error.The
extractDomainFromIdfunction returns an empty string on error (line 91), which could mask failures. Callers might not distinguish between "no domain" and "error extracting domain."Consider returning
nullor the originalmatrixIdon error for clearer error signaling:-export function extractDomainFromId(matrixId: string): string { +export function extractDomainFromId(matrixId: string): string | null { try { return extractDomainFromMatrixUserId(matrixId); } catch (error) { logger.error(`Error extracting domain from Matrix ID ${matrixId}:`, error); - return ''; + return null; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
apps/meteor/app/api/server/v1/rooms.ts(3 hunks)apps/meteor/app/lib/lib/MessageTypes.ts(1 hunks)apps/meteor/app/lib/server/functions/acceptRoomInvite.ts(1 hunks)apps/meteor/app/lib/server/functions/addUserToRoom.ts(3 hunks)apps/meteor/app/lib/server/functions/removeUserFromRoom.ts(2 hunks)apps/meteor/app/lib/server/methods/addUsersToRoom.ts(2 hunks)apps/meteor/ee/server/hooks/federation/index.ts(1 hunks)apps/meteor/lib/publishFields.ts(1 hunks)apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts(1 hunks)apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts(1 hunks)apps/meteor/server/modules/watchers/watchers.module.ts(1 hunks)apps/meteor/server/services/room/service.ts(2 hunks)ee/packages/federation-matrix/src/FederationMatrix.ts(4 hunks)ee/packages/federation-matrix/src/api/_matrix/invite.ts(3 hunks)ee/packages/federation-matrix/src/events/helpers.ts(1 hunks)ee/packages/federation-matrix/src/events/member.ts(1 hunks)ee/packages/federation-matrix/src/events/room.ts(1 hunks)ee/packages/federation-matrix/src/helpers/message.parsers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts
- apps/meteor/lib/publishFields.ts
- apps/meteor/app/lib/lib/MessageTypes.ts
- apps/meteor/server/modules/watchers/watchers.module.ts
- apps/meteor/app/api/server/v1/rooms.ts
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/app/lib/server/functions/acceptRoomInvite.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/ee/server/hooks/federation/index.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tsapps/meteor/server/services/room/service.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/lib/server/functions/acceptRoomInvite.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsapps/meteor/server/services/room/service.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/lib/server/functions/acceptRoomInvite.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsapps/meteor/server/services/room/service.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/lib/server/functions/acceptRoomInvite.tsee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/ee/server/hooks/federation/index.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tsapps/meteor/server/services/room/service.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
ee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/ee/server/hooks/federation/index.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/helpers.tsapps/meteor/app/lib/server/functions/addUserToRoom.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
ee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsapps/meteor/app/lib/server/functions/addUserToRoom.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/packages/federation-matrix/src/events/member.tsapps/meteor/ee/server/hooks/federation/index.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/helpers.ts
📚 Learning: 2025-10-01T12:20:13.147Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37110
File: apps/meteor/ee/server/startup/federation.ts:17-21
Timestamp: 2025-10-01T12:20:13.147Z
Learning: In Rocket.Chat's StreamerCentral broadcast for 'notify-room' events with 'user-activity', the `activity` argument is a string (e.g., 'user-typing') when a user starts typing and an empty array when the user stops typing.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.ts
🧬 Code graph analysis (7)
apps/meteor/app/lib/server/functions/acceptRoomInvite.ts (6)
apps/meteor/server/services/room/service.ts (1)
acceptRoomInvite(86-88)packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/core-typings/src/ISubscription.ts (1)
ISubscription(10-81)packages/core-typings/src/IUser.ts (1)
IUser(186-255)apps/meteor/lib/callbacks.ts (1)
callbacks(252-260)apps/meteor/app/lib/server/lib/notifyListener.ts (1)
notifyOnSubscriptionChangedById(535-544)
ee/packages/federation-matrix/src/events/member.ts (2)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
handleInvite(916-950)ee/packages/federation-matrix/src/events/helpers.ts (2)
getOrCreateFederatedUser(12-48)getOrCreateFederatedRoom(50-84)
apps/meteor/app/lib/server/methods/addUsersToRoom.ts (4)
packages/core-typings/src/IRoom.ts (1)
isRoomNativeFederated(124-125)ee/packages/federation-matrix/src/FederationMatrix.ts (1)
FederationMatrix(144-951)packages/core-services/src/index.ts (1)
FederationMatrix(199-199)apps/meteor/app/lib/server/functions/addUserToRoom.ts (1)
addUserToRoom(22-173)
apps/meteor/server/services/room/service.ts (2)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(10-81)apps/meteor/app/lib/server/functions/acceptRoomInvite.ts (1)
acceptRoomInvite(9-31)
ee/packages/federation-matrix/src/FederationMatrix.ts (3)
packages/core-typings/src/IRoom.ts (3)
IRoomNativeFederated(114-120)RoomID(12-12)isRoomNativeFederated(124-125)packages/core-typings/src/IUser.ts (2)
IUser(186-255)isUserNativeFederated(276-277)packages/core-typings/src/ISubscription.ts (1)
ISubscription(10-81)
ee/packages/federation-matrix/src/events/helpers.ts (4)
packages/core-typings/src/IUser.ts (1)
IUser(186-255)ee/packages/federation-matrix/src/FederationMatrix.ts (3)
getUsernameServername(80-92)createOrUpdateFederatedUser(99-140)extractDomainFromMatrixUserId(67-73)packages/core-typings/src/IRoom.ts (2)
RoomID(12-12)IRoom(21-95)packages/core-services/src/index.ts (1)
Room(169-169)
apps/meteor/app/lib/server/functions/addUserToRoom.ts (1)
packages/core-services/src/index.ts (1)
Message(184-184)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (20)
ee/packages/federation-matrix/src/helpers/message.parsers.ts (1)
5-5: MatrixMessageContent now correctly mirrors the federation SDK event content typeSwitching
MatrixMessageContenttoHomeserverEventSignatures['homeserver.matrix.message']['event']['content'] & { format?: string }aligns the local type with the SDK’s Matrix event shape and keepscreateReplyContentstrongly typed against the actualevent.contentpayload. The extra& { format?: string }is safe (and covers cases whereformatis not present on the SDK type) and doesn’t alter runtime behavior.apps/meteor/app/lib/server/functions/acceptRoomInvite.ts (1)
22-30: Join flow wiring looks consistent with existing patternsUsing
beforeJoinRoom/afterJoinRoomcallbacks, updating the subscription state, broadcasting vianotifyOnSubscriptionChangedById, and emitting a'uj'system message matches the existing room-join semantics, which should keep invite acceptance aligned with normal joins.apps/meteor/ee/server/hooks/federation/index.ts (1)
1-1: Imports are consistent with downstream usageThe
FederationMatrixandbeforeAddUsersToRoomimports line up with the usages later in this file (create room, message propagation, and ensuring federated users exist locally). The wiring here looks correct and keeps the federation hooks centralized.Also applies to: 8-8
apps/meteor/app/lib/server/functions/removeUserFromRoom.ts (1)
36-37: Nice separation of “reject invite” from normal leave/kick flowsIncluding
invitedin the projection and adding thesubscription.invitedbranch with the'uir'system message clearly distinguishes:
- Kick / removal by someone else (
options.byUserpresent),- Explicit invite rejection (
subscription.invited === true),- Normal user leave (
ul/ult).This should make invite rejection UX much clearer, as long as the reject paths call
removeUserFromRoomwithoutoptions.byUser. The implementation itself looks solid.Please double‑check that all “reject invite” call sites invoke
removeUserFromRoomwithoutoptions.byUser, so they hit the new'uir'branch instead of the generic leave/kick messages.Also applies to: 51-57
apps/meteor/app/lib/server/methods/addUsersToRoom.ts (1)
1-1: Federated invite integration intoaddUsersToRoomlooks correct; ensureaccess-federationpermission is still enforced elsewhereThe method now:
- Validates federated usernames upfront and rejects them for non‑federated rooms.
- Runs
beforeAddUsersToRoomonce for all usernames, passinginviter: user.- For users not yet subscribed:
- Optionally calls
FederationMatrix.inviteUsersToRoomwhen the room is native‑federated and the user has a username.- Threads
invitedandfederationmetadata through toaddUserToRoom, so subscription/system messages can reflect the invite.- For already‑subscribed users with
username, emits the ephemeral “already in here” message viaapi.broadcast.This is a good consolidation of the invite path into the existing add‑user flow.
Given the prior pattern in federation hooks (user‑initiated federation actions should enforce
access-federationand fail loudly, while remote events should be silent when lacking permission – based on learnings), just make sure that this method (or abeforeAddUsersToRoomsubscriber) still enforcesaccess-federationfor federated rooms when appropriate.If
access-federationenforcement has been moved to a shared callback (e.g., inbeforeAddUserToRoom.ts), please double‑check that it still runs for this method’s code path, and not for remote homeserver‑initiated events.Also applies to: 88-99, 101-109, 126-136
apps/meteor/server/services/room/service.ts (1)
86-88: LGTM! Clean delegation pattern for invite acceptance.The new method follows the established service pattern by delegating to the imported
acceptRoomInvitefunction. The signature is type-safe and consistent with other methods in the class.ee/packages/federation-matrix/src/api/_matrix/invite.ts (3)
136-172: LGTM! Validate local-only invite acceptance.The
acceptInvitefunction correctly validates that both inviter and invitee are local users (not native federated) when processing invites to federated rooms, which aligns with the comment stating this is for "inside rocket chat we invite users inside rockechat."
220-230: LGTM! Proper federation permission check added.The permission check correctly validates
access-federationbefore processing the invite, and logs appropriately when access is denied. This aligns with the federation permission patterns.Based on learnings
232-261: LGTM! Comprehensive error handling for invite processing.The error handling correctly distinguishes between
NotAllowedError(403) and general errors (500), providing appropriate error codes and messages for each case.ee/packages/federation-matrix/src/events/member.ts (3)
75-98: LGTM! Clean join handling with proper validation.The
handleJoinfunction correctly validates all prerequisites (user, room, subscription) before callingRoom.acceptRoomInvite. Error handling distinguishes between critical errors (missing room) and expected conditions (missing subscription for logging).
100-116: LGTM! Straightforward leave handling.The
handleLeavefunction correctly validates user and room existence before removing the user. The implementation is clean and follows consistent error logging patterns.
118-140: LGTM! Clean event dispatcher with proper error handling.The membership event dispatcher correctly routes to specialized handlers based on membership type, with appropriate error handling and logging. The
eventIdis passed only tohandleInvitewhere it's needed for federation metadata.apps/meteor/app/lib/server/functions/addUserToRoom.ts (2)
30-40: LGTM! Clean parameter extension for federation support.The optional
invitedandfederationparameters are well-typed and don't break existing callers. The federation object structure clearly captures the invite metadata needed for federation tracking.
109-110: LGTM! Clean conditional inclusion of federation fields.The subscription creation correctly uses conditional spread operators to include
invitedandfederationfields only when present, keeping the subscription document clean.ee/packages/federation-matrix/src/FederationMatrix.ts (3)
550-589: LGTM! Enhanced invite handling with proper user filtering.The updated
inviteUsersToRoommethod correctly filters users based on whether the inviter is native federated, preventing unnecessary processing. The new return type provides useful invite event context to callers.
859-862: LGTM! Important subscription verification added.The subscription check on lines 859-862 correctly verifies the user has joined the room before sending typing notifications. This prevents inappropriate typing notifications for users who haven't actually joined the room yet.
916-950: LGTM! Proper validation order for invite handling.The
handleInvitemethod correctly validates the subscription, user, and username (line 937-939) before constructing thematrixUserId(line 942). All validations are in the proper order to prevent errors.Note: A previous review comment suggested moving username validation before matrixUserId construction, but the current code already has the correct order.
ee/packages/federation-matrix/src/events/helpers.ts (3)
12-48: LGTM! Robust federated user resolution.The
getOrCreateFederatedUserhelper correctly distinguishes between local and remote users, only auto-creating remote federated users. Local users that don't exist trigger a warning and return null, which is the correct behavior.
50-84: LGTM! Clean federated room creation helper.The
getOrCreateFederatedRoomhelper follows a clean lookup-or-create pattern and correctly uses the Room service for creation. Federation metadata and room properties are properly configured.
95-103: LGTM! Safe username extraction with fallback.The
extractUsernameFromMatrixIdfunction safely extracts the username and returns the originalmatrixIdas a fallback on error, which is a reasonable approach.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/meteor/app/lib/server/functions/addUserToRoom.ts (1)
131-134: Validate inviter username before using it in system message.The
uisystem message path usesinviter.usernamewithout checking if it's defined. Sinceusernameis optional onIUser, this could result in an undefined value being stored in the message.Based on past review comments.
Apply this diff to add validation:
} else if (status === 'INVITED') { + if (!inviter.username) { + throw new Meteor.Error('error-invalid-inviter', 'Cannot create invite message without inviter username'); + } await Message.saveSystemMessage('ui', rid, userToBeAdded.username, userToBeAdded, { u: { _id: inviter._id, username: inviter.username }, });apps/meteor/app/lib/server/methods/addUsersToRoom.ts (1)
110-120: Validate inviter username before using it in invite options.Line 116 uses
user.usernamewithout checking if it's defined. Sinceusernameis optional onIUser, this could result in undefinedinviterUsernamebeing passed to the invite flow.Based on past review comments.
Apply this diff to add validation:
let inviteOptions: { status?: SubscriptionStatus; inviterUsername?: string } = {}; if (isRoomNativeFederated(room) && user && newUser.username) { + if (!user.username) { + throw new Meteor.Error('error-invalid-inviter', 'Cannot invite to federated room without inviter username'); + } await FederationMatrix.inviteUsersToRoom(room, [newUser.username], user); inviteOptions = { status: 'INVITED', inviterUsername: user.username, }; }ee/packages/federation-matrix/src/events/member.ts (1)
54-69: Validate inviter username before using it.Lines 59 and 68 use
inviterUser.usernamewithout checking if it's defined. Sinceusernameis optional onIUser, the cast toUserIDon line 59 is unsafe, and line 68 could store undefined asinviterUsername.Based on past review comments.
Apply this diff to add validation after line 18:
const inviterUser = await getOrCreateFederatedUser(senderId as UserID); if (!inviterUser) { logger.error(`Failed to get or create inviter user: ${senderId}`); return; } + if (!inviterUser.username) { + logger.error(`Inviter user ${inviterUser._id} has no username`); + return; + }
🧹 Nitpick comments (3)
ee/packages/federation-matrix/src/FederationMatrix.ts (3)
842-867: LGTM with minor optional refinement.The enhanced validation logic properly ensures the user has joined the room before sending typing notifications, preventing potential errors.
Optional refinement: Line 859 uses
localUser?._idwith optional chaining, but this is unnecessary since lines 855-857 already ensurelocalUserexists. You can simplify tolocalUser._id.- const hasUserJoinedRoom = await Subscriptions.findOneByRoomIdAndUserId(room._id, localUser?._id, { projection: { _id: 1 } }); + const hasUserJoinedRoom = await Subscriptions.findOneByRoomIdAndUserId(room._id, localUser._id, { projection: { _id: 1 } });
939-944: Conditional username validation needed for native federated users.The username validation at lines 939-941 occurs before constructing
matrixUserId(good!), but it should only be required when the user is NOT a native federated user. Native federated users useuser.federation.muidirectly and may not have a username set.Apply this diff to make username validation conditional:
const user = await Users.findOneById(userId); if (!user) { throw new Error('User not found'); } -if (!user.username) { - throw new Error('User username not found'); -} - // TODO: should use common function to get matrix user ID const matrixUserId = isUserNativeFederated(user) ? user.federation.mui : `@${user.username}:${this.serverName}`; + +if (!isUserNativeFederated(user) && !user.username) { + throw new Error('User username not found'); +}Based on learnings, this aligns with federation patterns where native federated users have different identifier requirements.
943-943: Address or track the TODO comment.The TODO suggests creating a common function to get the matrix user ID, which would improve code reusability since this pattern appears in multiple places (lines 466, 557, 610, 650, 699, 714, 747, 780, 795, 820, 864, 944).
Would you like me to open a new issue to track the creation of a common helper function for retrieving matrix user IDs?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/meteor/app/lib/server/functions/acceptRoomInvite.ts(1 hunks)apps/meteor/app/lib/server/functions/addUserToRoom.ts(4 hunks)apps/meteor/app/lib/server/functions/removeUserFromRoom.ts(2 hunks)apps/meteor/app/lib/server/methods/addUsersToRoom.ts(2 hunks)apps/meteor/lib/publishFields.ts(1 hunks)apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts(1 hunks)apps/meteor/server/modules/watchers/watchers.module.ts(1 hunks)ee/packages/federation-matrix/src/FederationMatrix.ts(4 hunks)ee/packages/federation-matrix/src/events/member.ts(1 hunks)packages/core-services/src/types/IRoomService.ts(2 hunks)packages/core-typings/src/IInquiry.ts(1 hunks)packages/core-typings/src/ISubscription.ts(2 hunks)packages/models/src/models/Subscriptions.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/meteor/app/lib/server/functions/removeUserFromRoom.ts
- packages/core-typings/src/ISubscription.ts
- apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts
- apps/meteor/app/lib/server/functions/acceptRoomInvite.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/models/src/models/Subscriptions.tspackages/core-typings/src/IInquiry.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/FederationMatrix.tspackages/core-services/src/types/IRoomService.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/lib/publishFields.tsapps/meteor/server/modules/watchers/watchers.module.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/FederationMatrix.tspackages/core-services/src/types/IRoomService.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/FederationMatrix.tspackages/core-services/src/types/IRoomService.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/FederationMatrix.tspackages/core-services/src/types/IRoomService.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/app/lib/server/functions/addUserToRoom.tsee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-10-01T12:20:13.147Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37110
File: apps/meteor/ee/server/startup/federation.ts:17-21
Timestamp: 2025-10-01T12:20:13.147Z
Learning: In Rocket.Chat's StreamerCentral broadcast for 'notify-room' events with 'user-activity', the `activity` argument is a string (e.g., 'user-typing') when a user starts typing and an empty array when the user stops typing.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/lib/publishFields.tsapps/meteor/server/modules/watchers/watchers.module.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL-Build
🔇 Additional comments (21)
apps/meteor/lib/publishFields.ts (1)
45-46: LGTM!The addition of
statusandinviterUsernamefields to the subscription broadcast projection is correct and aligns with the new invite flow support.packages/models/src/models/Subscriptions.ts (2)
164-164: LGTM!Filtering out subscriptions with a
statusfield ensures that invited users are not counted as active room members, which is the correct behavior for the invite flow.
2090-2104: LGTM!The
markInviteAsAcceptedmethod correctly transitions an invited subscription to an accepted state by unsetting invite-specific fields and opening the room for the user.packages/core-typings/src/IInquiry.ts (1)
64-69: LGTM!The enhanced type guard correctly distinguishes livechat inquiry records from regular subscriptions by validating that the
statusfield contains aLivechatInquiryStatusvalue, preventing false positives from subscriptions withSubscriptionStatusvalues.apps/meteor/server/modules/watchers/watchers.module.ts (1)
140-141: LGTM!Adding
statusandinviterUsernameto the subscription projection ensures these invite-related fields are properly broadcast to clients when subscriptions change.packages/core-services/src/types/IRoomService.ts (3)
1-1: LGTM!Import additions support the extended
addUserToRoomoptions and newacceptRoomInvitemethod signature.
42-43: LGTM!The optional
statusandinviterUsernamefields properly extend the options to support the federated invite flow while maintaining backward compatibility.
47-54: LGTM!The new
acceptRoomInvitemethod signature is well-defined and appropriate for handling the invite acceptance flow with optional system message suppression.apps/meteor/app/lib/server/functions/addUserToRoom.ts (3)
4-4: LGTM!Import addition supports the new
statusparameter for invite handling.
30-31: LGTM!The optional
statusandinviterUsernameparameters are properly typed and integrated into the function signature.Also applies to: 36-37
106-107: LGTM!The conditional spreading ensures
statusandinviterUsernameare only included in the subscription when defined, maintaining data integrity.apps/meteor/app/lib/server/methods/addUsersToRoom.ts (2)
1-2: LGTM!Import additions support the new federated room invite integration.
123-133: LGTM!The early return when
newUser.usernameis undefined prevents issues downstream, and the ephemeral message correctly uses the validated username.ee/packages/federation-matrix/src/events/member.ts (5)
11-24: LGTM!The inviter and invitee user resolution with early returns for missing users is a robust approach to error handling.
28-49: LGTM!The room type determination and naming logic correctly handles direct messages vs. group rooms, with the added check for
roomOriginDomainaddressing the previous review concern.
72-95: LGTM!The join handler correctly resolves the joining user, validates the room and subscription exist, and delegates to the room service for invite acceptance.
97-113: LGTM!The leave handler properly resolves the leaving user and room, with appropriate error logging for missing entities.
115-138: LGTM!The switch-based dispatcher provides clean routing to the appropriate membership handler with good error handling and logging for unknown membership types.
ee/packages/federation-matrix/src/FederationMatrix.ts (3)
10-20: LGTM!Type imports properly added to support the new invite handling functionality.
550-589: Verify the subscription status update after invite acceptance/rejection.The
inviteUsersToRoommethod now properly distinguishes between native federated and local inviters. However, ensure that when invites are sent, the corresponding subscription records are created or updated with the correct status (e.g., 'INVITED') and that acceptance/rejection flows properly update these records.
916-952: Verify subscription status updates after invite handling.The
handleInvitemethod correctly validates the subscription is in 'INVITED' status before processing, but ensure that accepting or rejecting the invite properly updates the subscription status (e.g., to 'JOINED' or removes the subscription). This might be handled by Matrix event handlers, but should be confirmed.
sampaiodiego
force-pushed
the
feat/invites
branch
from
3591e8f to
78f37b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
151-175: Inconsistent error handling:throwvs. structured response.Lines 154 and 174 throw plain
Errorobjects, while other error paths return structured responses witherrcodeandstatusCode. Thrown errors may not produce proper Matrix-compliant error responses unless caught by an outer handler.Consider returning structured error responses for consistency:
if (!userToCheck) { - throw new Error('join event has missing state key, unable to determine user to join'); + return { + body: { + errcode: 'M_MISSING_PARAM', + error: 'join event has missing state key, unable to determine user to join', + }, + statusCode: 400, + }; }if (!ourUser) { - throw new Error('user not found not processing invite'); + return { + body: { + errcode: 'M_NOT_FOUND', + error: 'User not found, not processing invite', + }, + statusCode: 404, + }; }
♻️ Duplicate comments (6)
apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts (1)
19-21: Type definition doesn't match runtime behavior.The
$unwindstage (lines 114-119) usespreserveNullAndEmptyArrays: true, meaning users without a subscription will havesubscriptionasundefined. However, the type declares it as required. This creates a type safety gap.Apply this diff to fix the type:
type UserWithRoleAndSubscriptionData = IUser & { - subscription: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>; + subscription?: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>; };apps/meteor/app/api/server/v1/rooms.ts (2)
1100-1105: Error handling pattern doesn't leverage response validators.The endpoint declares 400/401 response validators (lines 1085-1086) but the catch block returns a generic
API.v1.failure(). Consider letting errors propagate naturally so the framework handles them with proper response schemas, or differentiate error types to return correctly typed responses.Consider simplifying by removing the try-catch and letting the framework handle errors:
async function action() { const { roomId, action } = this.bodyParams; - - try { - await FederationMatrix.handleInvite(roomId, this.userId, action); - return API.v1.success(); - } catch (error) { - return API.v1.failure({ error: `Failed to handle invite: ${error instanceof Error ? error.message : String(error)}` }); - } + await FederationMatrix.handleInvite(roomId, this.userId, action); + return API.v1.success(); },
1097-1102: Parameter mismatch:roomIdpassed but interface expectssubscriptionId.The endpoint extracts
roomIdfrombodyParams(line 1098) and passes it toFederationMatrix.handleInvite, but the interface signature inIFederationMatrixService.tsdefines the first parameter assubscriptionId. Either the parameter name should besubscriptionIdin the request body, or the implementation/interface needs alignment.Verify the intended behavior and align the naming:
#!/bin/bash # Check the actual implementation of handleInvite to understand expected parameter ast-grep --pattern 'async handleInvite($_, $_, $_) { $$$ }'apps/meteor/ee/server/hooks/federation/index.ts (1)
105-137: Permission check is still applied to invited user instead of inviterIn this hook,
beforeAddUserToRoomruns only for local, user‑initiated actions, so theaccess-federationcheck should be enforced on the actor (inviter) rather than the target (user). The current code:if (!(await Authorization.hasPermission(user._id, 'access-federation'))) { throw new MeteorError('error-not-authorized-federation', 'Not authorized to access federation'); }allows an unauthorized inviter to invite someone who does have
access-federation, and blocks inviting a user who lacks the permission even if the inviter is authorized. Based on learnings, this violates the intended pattern for user‑initiated federation actions.Suggested fix:
if (!(await Authorization.hasPermission(inviter._id, 'access-federation'))) { throw new MeteorError('error-not-authorized-federation', 'Not authorized to access federation'); }(With a preceding guard ensuring
inviter._idexists.) This keeps the permission gate on the correct principal.apps/meteor/app/lib/server/functions/removeUserFromRoom.ts (1)
13-18: JSDoc still claims “no callbacks” but performUserRemoval runs beforeLeaveRoomCallbackThe comment on
performUserRemovalsays it executes “with no callbacks”, yet it invokesbeforeLeaveRoomCallback.run(user, room). That’s the same mismatch noted previously.Either update the JSDoc to mention that
beforeLeaveRoomCallbackis still executed (and that only app events / afterLeave are skipped on this path), or remove that callback fromperformUserRemovalif external/event-driven removals truly must avoid all callbacks.Also applies to: 26-28
ee/packages/federation-matrix/src/events/member.ts (1)
121-172: Guard against missingunsignedand undefined usernames in handleInviteTwo robustness issues here:
const strippedState = unsigned.invite_room_state;will throw ifunsignedis undefined. The membership event’sunsignedfield is optional, so this path should either use optional chaining or bail out cleanly ifinvite_room_stateis missing.
inviterUser.usernameandinviteeUser.usernameare optional onIUser, but they’re used without validation (including a force-cast oninviterUser.username as string). If either is undefined, you’ll build invalid members lists and federation metadata.Consider:
const strippedState = unsigned?.invite_room_state ?? []; if (!inviterUser.username || (content?.is_direct && !inviteeUser.username)) { logger.error(`Inviter or invitee username missing for invite in room ${roomId}`); return; }and then use the verified
usernamevalues withoutas stringcasts. This mirrors the username checks already present in other federation paths.
🧹 Nitpick comments (10)
apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts (1)
173-175: Consider adding'ui'and'uir'toitalicTypesfor consistent HTML export styling.System messages like
'uj','ul','au','ru', and'wm'are rendered in italics for HTML exports. The new invite-related message types ('ui','uir') are also system messages and may benefit from the same styling for consistency.- const italicTypes: IMessage['t'][] = ['uj', 'ul', 'au', 'r', 'ru', 'wm', 'livechat-close']; + const italicTypes: IMessage['t'][] = ['uj', 'ul', 'ui', 'uir', 'au', 'r', 'ru', 'wm', 'livechat-close'];apps/meteor/server/methods/createDirectMessage.ts (1)
76-88: The extraData pattern looks correct for federation callback mutation.The refactored approach passes
extraDataby reference to thefederation.beforeCreateDirectMessagecallback, allowing the hook to populate federation-related properties without returning values. This avoids the previous issue where callback returns could corrupt room data.One minor consideration: the empty object
{}lacks typing. If you want to be explicit about the expected shape, consider typing it asPartial<IRoom>to match how it's consumed downstream.- const extraData = {}; + const extraData: Partial<IRoom> = {};This would require importing
IRoomfrom@rocket.chat/core-typings.ee/packages/federation-matrix/src/api/_matrix/send-leave.ts (2)
13-46: Consider addingrequiredarray to nestedcontentobject schema.The
contentproperty has a nestedmembershipfield that should probably be required for proper validation of a leave event. Currently, the schema only defines the property but doesn't enforce its presence within thecontentobject.const isSendLeaveBodyProps = ajv.compile({ type: 'object', properties: { content: { type: 'object', properties: { membership: { type: 'string', const: 'leave', }, }, + required: ['membership'], }, depth: {
97-97: Inconsistent log message.The log message says "Error making leave" but this is the
send_leaveendpoint. Consider updating for clarity.- logger.error({ msg: 'Error making leave', err: error }); + logger.error({ msg: 'Error sending leave', err: error });ee/packages/federation-matrix/src/api/_matrix/make-leave.ts (1)
13-48: Success response schema missingrequiredarray.The success response schema defines
eventandroom_versionproperties but doesn't specify which are required. Consider adding arequiredarray to ensure proper validation of the SDK response.const isMakeLeaveSuccessResponseProps = ajv.compile({ type: 'object', properties: { event: { type: 'object', properties: { content: { type: 'object', properties: { membership: { type: 'string', const: 'leave', }, }, }, origin: { type: 'string', }, origin_server_ts: { type: 'number', }, sender: { type: 'string', }, state_key: { type: 'string', }, type: { type: 'string', const: 'm.room.member', }, }, }, room_version: { type: 'string' }, }, + required: ['event', 'room_version'], });apps/meteor/app/lib/server/functions/acceptRoomInvite.ts (1)
18-19: TODO comment indicates technical debt.The comment acknowledges code duplication with
addUserToRoom.ts. Consider creating a shared utility or extracting common logic to reduce maintenance burden in future iterations.Do you want me to open an issue to track this refactoring task?
apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts (1)
134-137: Mutating returned objects withdelete.The
mapcallback mutates the member objects by deletingrolePriority. This is a side-effect pattern that can cause issues if the same objects are referenced elsewhere. Consider using destructuring to return new objects without the unwanted property.return { - members: members.map((member: any) => { - delete member.rolePriority; - return member; - }), + members: members.map(({ rolePriority, ...member }) => member as UserWithRoleAndSubscriptionData), total: totalCount, };ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
743-744: Hardcoded delays may cause test flakiness.Multiple
setTimeoutcalls with fixed delays (e.g., 2000ms) are used to wait for federation setup. While acceptable for E2E tests, consider using polling with timeouts or explicit readiness checks if flakiness occurs in CI.apps/meteor/server/services/room/service.ts (1)
223-289: Centralized subscription creation logic looks correct; verify unread/mention semantics
createUserSubscriptioncorrectly consolidatesSubscriptions.createWithRoomAndUser, default prefs, autotranslate config, and the various system-message types (ui/au/ujetc.). The INVITED flow viastatus: 'INVITED'anduimessages matches the invite semantics.One thing to confirm:
unread: 1anduserMentions: 1are applied unconditionally, including INVITED subscriptions. If the intended UX is “one unread + mention” for invites and joins, this is fine; if not, consider conditioning these counters onstatusor a flag.ee/packages/federation-matrix/src/FederationMatrix.ts (1)
91-132: Align createOrUpdateFederatedUser return typing with projectionThe function is typed as returning
IUserbut thefindOneAndUpdateusesprojection: { _id: 1, username: 1 }, so callers won’t getfederated/federationfields even though they’re set in$set. Current call sites only use_id/username, but the widerIUserreturn type can be misleading.Consider either:
- Expanding the projection to include
federatedandfederation, or- Narrowing the return type to a
Pick<IUser, '_id' | 'username'>-style alias that matches what’s actually returned.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (41)
apps/meteor/app/api/server/v1/rooms.ts(3 hunks)apps/meteor/app/lib/lib/MessageTypes.ts(1 hunks)apps/meteor/app/lib/server/functions/acceptRoomInvite.ts(1 hunks)apps/meteor/app/lib/server/functions/addUserToRoom.ts(4 hunks)apps/meteor/app/lib/server/functions/createDirectRoom.ts(2 hunks)apps/meteor/app/lib/server/functions/createRoom.ts(6 hunks)apps/meteor/app/lib/server/functions/removeUserFromRoom.ts(4 hunks)apps/meteor/app/lib/server/methods/addUsersToRoom.ts(1 hunks)apps/meteor/ee/server/hooks/federation/index.ts(4 hunks)apps/meteor/lib/callbacks.ts(1 hunks)apps/meteor/lib/callbacks/beforeCreateRoomCallback.ts(1 hunks)apps/meteor/lib/publishFields.ts(1 hunks)apps/meteor/package.json(1 hunks)apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.ts(1 hunks)apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts(6 hunks)apps/meteor/server/methods/createDirectMessage.ts(1 hunks)apps/meteor/server/modules/watchers/watchers.module.ts(1 hunks)apps/meteor/server/services/room/service.ts(3 hunks)apps/meteor/tests/data/rooms.helper.ts(1 hunks)ee/packages/federation-matrix/package.json(1 hunks)ee/packages/federation-matrix/src/FederationMatrix.ts(9 hunks)ee/packages/federation-matrix/src/api/_matrix/invite.ts(3 hunks)ee/packages/federation-matrix/src/api/_matrix/make-leave.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/send-leave.ts(1 hunks)ee/packages/federation-matrix/src/api/routes.ts(2 hunks)ee/packages/federation-matrix/src/events/member.ts(1 hunks)ee/packages/federation-matrix/tests/end-to-end/room.spec.ts(13 hunks)packages/apps-engine/src/definition/messages/MessageType.ts(1 hunks)packages/core-services/package.json(1 hunks)packages/core-services/src/types/IFederationMatrixService.ts(2 hunks)packages/core-services/src/types/IRoomService.ts(4 hunks)packages/core-typings/src/IMessage/IMessage.ts(1 hunks)packages/core-typings/src/IRoom.ts(1 hunks)packages/core-typings/src/ISubscription.ts(2 hunks)packages/i18n/src/locales/en.i18n.json(2 hunks)packages/message-types/src/registrations/common.ts(1 hunks)packages/model-typings/src/models/IRoomsModel.ts(1 hunks)packages/model-typings/src/models/ISubscriptionsModel.ts(1 hunks)packages/models/src/models/Rooms.ts(1 hunks)packages/models/src/models/Subscriptions.ts(2 hunks)packages/rest-typings/src/v1/rooms.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/core-services/package.json
- apps/meteor/lib/callbacks.ts
- apps/meteor/app/lib/lib/MessageTypes.ts
- packages/models/src/models/Rooms.ts
- apps/meteor/app/lib/server/functions/createDirectRoom.ts
- packages/message-types/src/registrations/common.ts
- packages/model-typings/src/models/IRoomsModel.ts
- packages/i18n/src/locales/en.i18n.json
- apps/meteor/lib/publishFields.ts
- packages/apps-engine/src/definition/messages/MessageType.ts
- packages/core-typings/src/IMessage/IMessage.ts
- apps/meteor/package.json
- apps/meteor/tests/data/rooms.helper.ts
- packages/core-typings/src/ISubscription.ts
- apps/meteor/app/lib/server/methods/addUsersToRoom.ts
- apps/meteor/lib/callbacks/beforeCreateRoomCallback.ts
- ee/packages/federation-matrix/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/core-typings/src/IRoom.tspackages/model-typings/src/models/ISubscriptionsModel.tsee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/api/_matrix/invite.tspackages/models/src/models/Subscriptions.tsapps/meteor/server/modules/watchers/watchers.module.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsapps/meteor/server/lib/dataExport/exportRoomMessagesToFile.tsapps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/lib/server/functions/acceptRoomInvite.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tspackages/rest-typings/src/v1/rooms.tspackages/core-services/src/types/IRoomService.tsapps/meteor/app/lib/server/functions/createRoom.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
🧠 Learnings (26)
📓 Common learnings
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
packages/core-typings/src/IRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsapps/meteor/server/lib/dataExport/exportRoomMessagesToFile.tsapps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/lib/server/functions/acceptRoomInvite.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tspackages/rest-typings/src/v1/rooms.tspackages/core-services/src/types/IRoomService.tsapps/meteor/app/lib/server/functions/createRoom.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
packages/core-typings/src/IRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tsapps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tsapps/meteor/app/lib/server/functions/createRoom.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
packages/core-typings/src/IRoom.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsapps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/lib/server/functions/acceptRoomInvite.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tspackages/rest-typings/src/v1/rooms.tsapps/meteor/app/lib/server/functions/createRoom.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/model-typings/src/models/ISubscriptionsModel.tsapps/meteor/server/modules/watchers/watchers.module.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsapps/meteor/server/services/room/service.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
packages/model-typings/src/models/ISubscriptionsModel.tspackages/models/src/models/Subscriptions.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/lib/server/functions/acceptRoomInvite.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tspackages/rest-typings/src/v1/rooms.tspackages/core-services/src/types/IRoomService.tsapps/meteor/app/lib/server/functions/createRoom.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
packages/model-typings/src/models/ISubscriptionsModel.tspackages/models/src/models/Subscriptions.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/lib/server/functions/acceptRoomInvite.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tspackages/rest-typings/src/v1/rooms.tspackages/core-services/src/types/IRoomService.tsapps/meteor/app/lib/server/functions/createRoom.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
packages/model-typings/src/models/ISubscriptionsModel.tspackages/models/src/models/Subscriptions.tsapps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/lib/server/functions/acceptRoomInvite.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tspackages/rest-typings/src/v1/rooms.tspackages/core-services/src/types/IRoomService.tsapps/meteor/app/lib/server/functions/createRoom.tsapps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/api/_matrix/send-leave.tsee/packages/federation-matrix/src/api/_matrix/make-leave.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/packages/federation-matrix/src/api/routes.tsee/packages/federation-matrix/src/api/_matrix/invite.tsapps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/createRoom.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/server/lib/dataExport/exportRoomMessagesToFile.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/methods/createDirectMessage.tsee/packages/federation-matrix/src/events/member.tspackages/rest-typings/src/v1/rooms.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/app/api/server/v1/rooms.tsapps/meteor/server/methods/createDirectMessage.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/app/lib/server/functions/acceptRoomInvite.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/addUserToRoom.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.tspackages/rest-typings/src/v1/rooms.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsapps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/server/methods/createDirectMessage.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Applied to files:
apps/meteor/server/methods/createDirectMessage.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/app/lib/server/functions/removeUserFromRoom.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Applied to files:
apps/meteor/server/methods/createDirectMessage.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
ee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-09-30T13:00:05.465Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 36990
File: apps/meteor/ee/server/apps/storage/AppRealStorage.ts:55-58
Timestamp: 2025-09-30T13:00:05.465Z
Learning: In AppRealStorage (apps/meteor/ee/server/apps/storage/AppRealStorage.ts), the `remove` method is designed to be idempotent and returns `{ success: true }` unconditionally because the goal is to ensure the app is removed, not to distinguish whether this specific call performed the deletion. Database errors will throw exceptions.
Applied to files:
apps/meteor/app/lib/server/functions/removeUserFromRoom.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.ts
🧬 Code graph analysis (17)
packages/model-typings/src/models/ISubscriptionsModel.ts (1)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-79)
ee/packages/federation-matrix/src/api/routes.ts (2)
ee/packages/federation-matrix/src/api/_matrix/send-leave.ts (1)
getMatrixSendLeaveRoutes(60-109)ee/packages/federation-matrix/src/api/_matrix/make-leave.ts (1)
getMatrixMakeLeaveRoutes(58-106)
packages/models/src/models/Subscriptions.ts (1)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-79)
ee/packages/federation-matrix/src/api/_matrix/send-leave.ts (2)
packages/rest-typings/src/v1/Ajv.ts (1)
ajv(24-24)ee/packages/federation-matrix/src/api/middlewares/isAuthenticated.ts (1)
isAuthenticatedMiddleware(5-39)
apps/meteor/app/api/server/v1/rooms.ts (5)
apps/meteor/app/api/server/index.ts (1)
API(55-55)packages/rest-typings/src/v1/rooms.ts (1)
isRoomsInviteProps(712-712)packages/rest-typings/src/v1/Ajv.ts (3)
validateBadRequestErrorResponse(47-47)validateUnauthorizedErrorResponse(70-70)ajv(24-24)ee/packages/federation-matrix/src/FederationMatrix.ts (1)
FederationMatrix(136-929)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(73-77)
apps/meteor/server/methods/createDirectMessage.ts (2)
apps/meteor/app/lib/server/functions/createRoom.ts (1)
createRoom(144-325)packages/core-typings/src/IUser.ts (1)
IUser(186-256)
packages/core-services/src/types/IFederationMatrixService.ts (3)
packages/core-typings/src/IRoom.ts (1)
IRoomFederated(109-112)packages/core-typings/src/IUser.ts (1)
IUser(186-256)packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-79)
apps/meteor/app/lib/server/functions/acceptRoomInvite.ts (6)
apps/meteor/server/services/room/service.ts (1)
performAcceptRoomInvite(93-95)packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-79)packages/core-typings/src/IUser.ts (1)
IUser(186-256)apps/meteor/lib/callbacks.ts (1)
callbacks(252-260)apps/meteor/app/lib/server/lib/notifyListener.ts (1)
notifyOnSubscriptionChangedById(535-544)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (2)
apps/meteor/tests/data/rooms.helper.ts (3)
getGroupHistory(343-358)acceptRoomInvite(439-455)rejectRoomInvite(468-484)packages/core-typings/src/IMessage/IMessage.ts (1)
IMessage(164-262)
ee/packages/federation-matrix/src/events/member.ts (1)
ee/packages/federation-matrix/src/FederationMatrix.ts (2)
getUsernameServername(72-84)createOrUpdateFederatedUser(91-132)
apps/meteor/app/lib/server/functions/addUserToRoom.ts (2)
packages/core-typings/src/IRoom.ts (1)
isRoomNativeFederated(125-126)packages/core-services/src/index.ts (1)
Room(169-169)
packages/rest-typings/src/v1/rooms.ts (2)
packages/core-typings/src/IUser.ts (1)
IUser(186-256)packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-79)
packages/core-services/src/types/IRoomService.ts (3)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/core-typings/src/IUser.ts (1)
IUser(186-256)packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-79)
apps/meteor/app/lib/server/functions/createRoom.ts (3)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
FederationMatrix(136-929)ee/packages/federation-matrix/src/index.ts (1)
FederationMatrix(3-3)packages/core-services/src/index.ts (2)
FederationMatrix(199-199)Room(169-169)
ee/packages/federation-matrix/src/FederationMatrix.ts (3)
packages/core-typings/src/IUser.ts (2)
IUser(186-256)isUserNativeFederated(277-278)packages/core-typings/src/IRoom.ts (2)
IRoom(21-95)isRoomNativeFederated(125-126)packages/core-services/src/index.ts (1)
Room(169-169)
apps/meteor/server/services/room/service.ts (7)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/core-typings/src/IUser.ts (1)
IUser(186-256)apps/meteor/app/lib/server/functions/removeUserFromRoom.ts (1)
performUserRemoval(18-68)packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-79)apps/meteor/app/lib/server/functions/acceptRoomInvite.ts (1)
performAcceptRoomInvite(20-61)apps/meteor/app/lib/server/lib/notifyListener.ts (1)
notifyOnSubscriptionChangedById(535-544)packages/core-services/src/index.ts (1)
Message(184-184)
apps/meteor/ee/server/hooks/federation/index.ts (5)
ee/packages/federation-matrix/src/events/room.ts (1)
room(8-88)packages/core-typings/src/IRoom.ts (3)
isRoomNativeFederated(125-126)IRoom(21-95)IRoomNativeFederated(114-121)ee/packages/federation-matrix/src/events/member.ts (1)
member(240-263)packages/core-typings/src/IUser.ts (2)
isUserNativeFederated(277-278)IUser(186-256)apps/meteor/lib/callbacks/beforeCreateRoomCallback.ts (1)
prepareCreateRoomCallback(8-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
| 200: ajv.compile<void>({ | ||
| type: 'object', | ||
| properties: { | ||
| success: { type: 'boolean', enum: [true] }, | ||
| }, | ||
| required: ['success'], | ||
| additionalProperties: false, | ||
| }), |
There was a problem hiding this comment.
Response schema declares void but returns object with success.
The 200 response validator (line 1087) is typed as ajv.compile<void> but the schema defines an object with a success boolean property. The generic type should match the schema.
- 200: ajv.compile<void>({
+ 200: ajv.compile<{ success: true }>({
type: 'object',
properties: {
success: { type: 'boolean', enum: [true] },
},
required: ['success'],
additionalProperties: false,
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 200: ajv.compile<void>({ | |
| type: 'object', | |
| properties: { | |
| success: { type: 'boolean', enum: [true] }, | |
| }, | |
| required: ['success'], | |
| additionalProperties: false, | |
| }), | |
| 200: ajv.compile<{ success: true }>({ | |
| type: 'object', | |
| properties: { | |
| success: { type: 'boolean', enum: [true] }, | |
| }, | |
| required: ['success'], | |
| additionalProperties: false, | |
| }), |
🤖 Prompt for AI Agents
In apps/meteor/app/api/server/v1/rooms.ts around lines 1087 to 1094, the 200
response ajv.compile is typed as ajv.compile<void> but the JSON schema defines
an object with a success:boolean property; update the generic to match the
schema (e.g. ajv.compile<{ success: boolean }>) so the TypeScript type reflects
the validated response, and ensure any callers/return types align with the new
typed shape.
| if (room.t === 'c' || room.t === 'p') { | ||
| process.nextTick(async () => { | ||
| // Add a new event, with an optional inviter | ||
| await callbacks.run('afterAddedToRoom', { user, inviter }, room); | ||
|
|
||
| // Keep the current event | ||
| await callbacks.run('afterJoinRoom', user, room); | ||
|
|
||
| void Apps.self?.triggerEvent(AppEvents.IPostRoomUserJoined, room, user, inviter); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Unhandled promise rejections in deferred callbacks.
The process.nextTick callback runs async operations but errors within it are not caught. If any callback or app event fails, the error will become an unhandled promise rejection.
Consider wrapping the deferred logic in a try-catch:
if (room.t === 'c' || room.t === 'p') {
process.nextTick(async () => {
- // Add a new event, with an optional inviter
- await callbacks.run('afterAddedToRoom', { user, inviter }, room);
-
- // Keep the current event
- await callbacks.run('afterJoinRoom', user, room);
-
- void Apps.self?.triggerEvent(AppEvents.IPostRoomUserJoined, room, user, inviter);
+ try {
+ await callbacks.run('afterAddedToRoom', { user, inviter }, room);
+ await callbacks.run('afterJoinRoom', user, room);
+ void Apps.self?.triggerEvent(AppEvents.IPostRoomUserJoined, room, user, inviter);
+ } catch (error) {
+ // Log error but don't propagate since this is a deferred callback
+ console.error('Error in post-join callbacks:', error);
+ }
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (room.t === 'c' || room.t === 'p') { | |
| process.nextTick(async () => { | |
| // Add a new event, with an optional inviter | |
| await callbacks.run('afterAddedToRoom', { user, inviter }, room); | |
| // Keep the current event | |
| await callbacks.run('afterJoinRoom', user, room); | |
| void Apps.self?.triggerEvent(AppEvents.IPostRoomUserJoined, room, user, inviter); | |
| }); | |
| } | |
| if (room.t === 'c' || room.t === 'p') { | |
| process.nextTick(async () => { | |
| try { | |
| await callbacks.run('afterAddedToRoom', { user, inviter }, room); | |
| await callbacks.run('afterJoinRoom', user, room); | |
| void Apps.self?.triggerEvent(AppEvents.IPostRoomUserJoined, room, user, inviter); | |
| } catch (error) { | |
| console.error('Error in post-join callbacks:', error); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/meteor/app/lib/server/functions/acceptRoomInvite.ts around lines 50–60,
the process.nextTick callback executes async operations without error handling
causing possible unhandled promise rejections; wrap the entire async body in a
try/catch, catch and handle/log any errors from callbacks.run and
Apps.self.triggerEvent (do not let them bubble), and ensure the catch logs the
error context (which room/user/inviter) and prevents an unhandled rejection.
| }, | ||
| }, | ||
| required: ['type', 'content', 'sender', 'room_id', 'origin_server_ts', 'depth', 'prev_events', 'auth_events', 'origin'], | ||
| required: ['type', 'content', 'sender', 'room_id', 'origin_server_ts', 'depth', 'prev_events', 'auth_events'], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n ee/packages/federation-matrix/src/api/_matrix/invite.ts | head -80 | tail -30Repository: RocketChat/Rocket.Chat
Length of output: 824
🏁 Script executed:
# Let's see the full schema definition context
cat -n ee/packages/federation-matrix/src/api/_matrix/invite.tsRepository: RocketChat/Rocket.Chat
Length of output: 6549
🏁 Script executed:
# Search for EventBaseSchema definition
rg "EventBaseSchema" ee/packages/federation-matrix --type ts -B 5 -A 10Repository: RocketChat/Rocket.Chat
Length of output: 8918
🏁 Script executed:
# Check if there are schema files in the federation-matrix package
fd "schema\|event" ee/packages/federation-matrix/src --type fRepository: RocketChat/Rocket.Chat
Length of output: 48
🌐 Web query:
Matrix federation specification event origin field required member
💡 Result:
Short answer: Yes — the federation spec requires an "origin" field. The Server‑Server API defines "origin" as a required string in federation transactions and in join/make_join/send_join payloads (it must be the sending/resident homeserver name). [1][2]
🏁 Script executed:
# Check git history to see if origin was deliberately removed from required
git log --oneline -p ee/packages/federation-matrix/src/api/_matrix/invite.ts | head -100Repository: RocketChat/Rocket.Chat
Length of output: 2283
🏁 Script executed:
# Check other event schemas in the codebase to understand the pattern
rg "required.*origin" ee/packages/federation-matrix --type ts -B 5 -A 2Repository: RocketChat/Rocket.Chat
Length of output: 6060
🏁 Script executed:
# Double-check the send-join schema to confirm origin is required there
sed -n '1,150p' ee/packages/federation-matrix/src/api/_matrix/send-join.ts | grep -A 60 "EventBaseSchema = {"Repository: RocketChat/Rocket.Chat
Length of output: 896
Add origin to the required fields array in EventBaseSchema.
The origin field is defined in the schema but not marked as required, which violates the Matrix federation specification and is inconsistent with other similar event schemas in this package (send-join.ts, send-leave.ts, transactions.ts, profiles.ts all require origin). Update line 65 to include 'origin' in the required array.
🤖 Prompt for AI Agents
In ee/packages/federation-matrix/src/api/_matrix/invite.ts around line 65, the
EventBaseSchema required array is missing the 'origin' field; add 'origin' to
the required list so the schema matches the Matrix federation spec and other
event schemas (e.g., send-join.ts), ensuring incoming events must include
origin.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/app/lib/server/functions/createRoom.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/lib/server/functions/createRoom.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/app/lib/server/functions/createRoom.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/lib/server/functions/createRoom.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/app/lib/server/functions/createRoom.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/app/lib/server/functions/createRoom.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/lib/server/functions/createRoom.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/lib/server/functions/createRoom.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
apps/meteor/app/lib/server/functions/createRoom.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
apps/meteor/app/lib/server/functions/createRoom.ts (6)
3-3: LGTM!The new imports for
FederationMatrix,Message,Room, andTeamfrom@rocket.chat/core-servicesare all utilized in the file.
73-80: LGTM!The subscription creation with
INVITEDstatus and properinvitermetadata aligns well with the PR objectives for federation invite handling.
158-158: LGTM!The
extraDatahandling is consistent throughout the function. The federation checks (extraData.federated === true) properly guard against mixing federated users with non-federated rooms and correctly determine when federation handling is needed.Also applies to: 169-169, 185-186
194-194: LGTM!Using
owner?._idas the fallback forcreatoris correct since creator should be an ID. The optional chaining handles the case whereownermay be undefined for direct rooms.
299-304: Callback naming inconsistency acknowledged.The comment explains why
federation.afterCreateFederatedRoomruns before subscriptions (Matrix room must exist for invites). The naming inconsistency is noted by the developer as something to discuss. The logic itself is correct for the federation invite flow.
306-306: LGTM!The call to
createUsersSubscriptionscorrectly passes all required parameters including the newshouldBeHandledByFederationflag.
| // Invite federated members to the room SYNCRONOUSLY, | ||
| // since we do not use to invite lots of users at once, this is acceptable. | ||
| const membersToInvite = members.filter((m) => m !== owner.username); | ||
|
|
||
| await FederationMatrix.ensureFederatedUsersExistLocally(membersToInvite); | ||
|
|
||
| for await (const memberUsername of membersToInvite) { | ||
| const member = await Users.findOneByUsername(memberUsername); | ||
| if (!member) { | ||
| throw new Error('Federated user not found locally'); | ||
| } |
There was a problem hiding this comment.
Inconsistent error type and typo.
- "SYNCRONOUSLY" should be "SYNCHRONOUSLY".
- The error thrown is a generic
Errorwhile the rest of the file usesMeteor.Error. This inconsistency may cause issues with error handling middleware that expectsMeteor.Error.
- // Invite federated members to the room SYNCRONOUSLY,
- // since we do not use to invite lots of users at once, this is acceptable.
+ // Invite federated members to the room SYNCHRONOUSLY,
+ // since we do not use to invite lots of users at once, this is acceptable.
const membersToInvite = members.filter((m) => m !== owner.username);
await FederationMatrix.ensureFederatedUsersExistLocally(membersToInvite);
for await (const memberUsername of membersToInvite) {
const member = await Users.findOneByUsername(memberUsername);
if (!member) {
- throw new Error('Federated user not found locally');
+ throw new Meteor.Error('error-federated-user-not-found', 'Federated user not found locally', {
+ method: 'createRoom',
+ });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Invite federated members to the room SYNCRONOUSLY, | |
| // since we do not use to invite lots of users at once, this is acceptable. | |
| const membersToInvite = members.filter((m) => m !== owner.username); | |
| await FederationMatrix.ensureFederatedUsersExistLocally(membersToInvite); | |
| for await (const memberUsername of membersToInvite) { | |
| const member = await Users.findOneByUsername(memberUsername); | |
| if (!member) { | |
| throw new Error('Federated user not found locally'); | |
| } | |
| // Invite federated members to the room SYNCHRONOUSLY, | |
| // since we do not use to invite lots of users at once, this is acceptable. | |
| const membersToInvite = members.filter((m) => m !== owner.username); | |
| await FederationMatrix.ensureFederatedUsersExistLocally(membersToInvite); | |
| for await (const memberUsername of membersToInvite) { | |
| const member = await Users.findOneByUsername(memberUsername); | |
| if (!member) { | |
| throw new Meteor.Error('error-federated-user-not-found', 'Federated user not found locally', { | |
| method: 'createRoom', | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/meteor/app/lib/server/functions/createRoom.ts around lines 61 to 71, fix
the typo "SYNCRONOUSLY" -> "SYNCHRONOUSLY" and replace the thrown generic Error
with a Meteor.Error so it matches the rest of the file’s error handling; update
the throw to use new Meteor.Error with an appropriate error code and descriptive
message (e.g. 'federated-user-not-found', 'Federated user not found locally') so
middleware expecting Meteor.Error will handle it correctly.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
apps/meteor/ee/server/hooks/federation/index.ts (1)
115-118: Permission check still applied to invited user instead of inviter.The permission check on line 116 validates
user._id(the invited user) rather thaninviter._id(the actor). This was previously flagged and marked as addressed, but the current code still checks the wrong user ID.Per retrieved learnings, user-initiated federation actions should validate the initiating user's permissions, not the target user's.
Apply this diff:
- if (!(await Authorization.hasPermission(user._id, 'access-federation'))) { + if (!(await Authorization.hasPermission(inviter._id, 'access-federation'))) { throw new MeteorError('error-not-authorized-federation', 'Not authorized to access federation'); }
🧹 Nitpick comments (1)
apps/meteor/ee/server/hooks/federation/index.ts (1)
272-272: Consider defensive null-check for room.federation.While
FederationActions.shouldPerformFederationAction(room)andprepareCreateRoomCallbackshould ensureroom.federationexists, using optional chaining would make this code more resilient to unexpected state.Apply this diff:
- if (!room.federation.mrid) { + if (!room.federation?.mrid) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/ee/server/hooks/federation/index.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/ee/server/hooks/federation/index.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.ts
🧬 Code graph analysis (1)
apps/meteor/ee/server/hooks/federation/index.ts (4)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
FederationMatrix(136-929)packages/core-typings/src/IRoom.ts (3)
isRoomNativeFederated(125-126)IRoom(21-95)IRoomNativeFederated(114-121)packages/core-typings/src/IUser.ts (2)
isUserNativeFederated(277-278)IUser(186-256)apps/meteor/lib/callbacks/beforeCreateRoomCallback.ts (1)
prepareCreateRoomCallback(8-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
| } else { | ||
| // TODO unify how to get server | ||
| // matrix room was already created and passed | ||
| const fromServer = federatedRoomId.split(':')[1]; |
There was a problem hiding this comment.
Validate federatedRoomId format before extracting server.
The code assumes federatedRoomId follows the format !id:server and directly accesses split(':')[1]. If the ID is malformed or doesn't contain a colon, this will return undefined, potentially causing Rooms.setAsFederated to receive an invalid origin value.
Apply this diff to add validation:
- const fromServer = federatedRoomId.split(':')[1];
+ const parts = federatedRoomId.split(':');
+ if (parts.length < 2) {
+ throw new MeteorError('error-invalid-federation-id', 'Invalid federated room ID format');
+ }
+ const fromServer = parts[1];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fromServer = federatedRoomId.split(':')[1]; | |
| const parts = federatedRoomId.split(':'); | |
| if (parts.length < 2) { | |
| throw new MeteorError('error-invalid-federation-id', 'Invalid federated room ID format'); | |
| } | |
| const fromServer = parts[1]; |
🤖 Prompt for AI Agents
In apps/meteor/ee/server/hooks/federation/index.ts around line 32, the code
unconditionally does const fromServer = federatedRoomId.split(':')[1]; which can
yield undefined for malformed IDs; validate the federatedRoomId contains a colon
and at least two parts before extracting the server part. If validation fails,
handle it by logging an error and aborting the operation (or returning early) so
Rooms.setAsFederated is not called with an invalid origin; otherwise assign
fromServer from the second split part and proceed.
| beforeAddUsersToRoom.add(async ({ usernames, inviter }, room) => { | ||
| if (!FederationActions.shouldPerformFederationAction(room) && inviter) { | ||
| // check if trying to invite a federated user to a non-federated room | ||
| const federatedUsernames = usernames.filter((u) => validateFederatedUsername(u)); | ||
| if (federatedUsernames.length > 0) { | ||
| throw new MeteorError('error-federated-users-in-non-federated-rooms', 'Cannot add federated users to non-federated rooms'); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // we create local users before adding them to the room | ||
| await FederationMatrix.ensureFederatedUsersExistLocally(usernames); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the beforeAddUsersToRoom hook definition and usage patterns
rg -n 'beforeAddUsersToRoom' apps/meteor/ee/server/hooks/federation/ -A 5 -B 2Repository: RocketChat/Rocket.Chat
Length of output: 2087
🏁 Script executed:
#!/bin/bash
# Search for calls to the hook callback or room event triggers with usernames
rg -n 'addUserToRoom|addUsersToRoom' apps/meteor/ee/server/ --type ts | head -30Repository: RocketChat/Rocket.Chat
Length of output: 501
🏁 Script executed:
#!/bin/bash
# Look for federation-related user addition patterns
rg -n 'inviter\s*:' apps/meteor/ee/server/hooks/federation/ --type ts -B 3 -A 3Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check the Room model to understand addUsersToRoom method
fd -e ts -e js 'room' apps/meteor | xargs rg -l 'addUsersToRoom' | head -10Repository: RocketChat/Rocket.Chat
Length of output: 209
🏁 Script executed:
#!/bin/bash
# Check the addUsersToRoom method implementation
cat -n apps/meteor/app/lib/server/methods/addUsersToRoom.ts | head -60Repository: RocketChat/Rocket.Chat
Length of output: 2484
🏁 Script executed:
#!/bin/bash
# Also check addUserToRoom for comparison
cat -n apps/meteor/app/lib/server/methods/addUserToRoom.ts | head -80Repository: RocketChat/Rocket.Chat
Length of output: 938
🏁 Script executed:
#!/bin/bash
# Search for beforeAddUsersToRoom callback definition to understand parameters
rg -n 'beforeAddUsersToRoom.*=' apps/meteor/lib/callbacks/ --type ts -B 2 -A 5Repository: RocketChat/Rocket.Chat
Length of output: 559
🏁 Script executed:
#!/bin/bash
# Continue reading addUsersToRoomMethod to find the callback invocation
cat -n apps/meteor/app/lib/server/methods/addUsersToRoom.ts | head -150 | tail -100Repository: RocketChat/Rocket.Chat
Length of output: 3539
🏁 Script executed:
#!/bin/bash
# Search for beforeAddUsersToRoom.run in the file
rg -n 'beforeAddUsersToRoom' apps/meteor/app/lib/server/methods/addUsersToRoom.ts -B 5 -A 5Repository: RocketChat/Rocket.Chat
Length of output: 982
🏁 Script executed:
#!/bin/bash
# Search for all calls to addUsersToRoomMethod to see if user is ever undefined
rg -n 'addUsersToRoomMethod' apps/meteor/ --type ts -B 2 -A 2Repository: RocketChat/Rocket.Chat
Length of output: 8228
🏁 Script executed:
#!/bin/bash
# Check if there are other places where beforeAddUsersToRoom is registered
rg -n 'beforeAddUsersToRoom.add\|beforeAddUsersToRoom.run' apps/meteor/ --type tsRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Also check the federation hook more carefully - check if there's specific handling for system actions
cat -n apps/meteor/ee/server/hooks/federation/index.ts | head -110 | tail -25Repository: RocketChat/Rocket.Chat
Length of output: 1046
🏁 Script executed:
#!/bin/bash
# Check slashcommands-inviteall more carefully
sed -n '80,100p' apps/meteor/app/slashcommands-inviteall/server/server.tsRepository: RocketChat/Rocket.Chat
Length of output: 803
🏁 Script executed:
#!/bin/bash
# Check slashcommands-invite more carefully
sed -n '90,110p' apps/meteor/app/slashcommands-invite/server/server.tsRepository: RocketChat/Rocket.Chat
Length of output: 519
🏁 Script executed:
#!/bin/bash
# Check bot-helpers more carefully
sed -n '75,95p' apps/meteor/app/bot-helpers/server/index.tsRepository: RocketChat/Rocket.Chat
Length of output: 621
Pass inviter to all addUsersToRoomMethod calls to enforce federation guards consistently.
The condition !FederationActions.shouldPerformFederationAction(room) && inviter relies on inviter being present, but several callers omit the third parameter: bot-helpers (line 85) and slashcommands-inviteall (line 93). When inviter is undefined, the federation validation bypass and federated users can be added to non-federated rooms. All calls to addUsersToRoomMethod should pass the inviter parameter to ensure the federation security check is consistently applied.
🤖 Prompt for AI Agents
In apps/meteor/ee/server/hooks/federation/index.ts around lines 91–103, the
federation guard checks rely on the inviter parameter but some callers omit it;
update all calls to addUsersToRoomMethod (notably in bot-helpers and
slashcommands-inviteall) to pass the inviter argument so the federation
validation runs consistently. Locate those caller sites and forward the current
user/inviter (e.g., the user object or userId used in the caller) as the third
parameter to addUsersToRoomMethod; if the caller only has a userId, pass the
corresponding user or ensure addUsersToRoomMethod accepts the same inviter shape
used by the hook.
| // TODO: use a shared helper to check whether a user is federated | ||
| // since the DM creation API doesn't tell us if the room is federated (unlike normal channels), | ||
| // we're currently inferring it: if any participant has a Matrix-style ID (@user:server), we treat the DM as federated | ||
| const hasFederatedMembers = roomUsers.some((user: unknown) => typeof user === 'string' && user.includes(':') && user.includes('@')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use validateFederatedUsername for consistency.
The check typeof user === 'string' && user.includes(':') && user.includes('@') is less strict than validateFederatedUsername used elsewhere in this file (line 94). This could incorrectly classify usernames like "test:@" as federated.
Apply this diff to use the imported validator:
- const hasFederatedMembers = roomUsers.some((user: unknown) => typeof user === 'string' && user.includes(':') && user.includes('@'));
+ const hasFederatedMembers = roomUsers.some((user: unknown) => typeof user === 'string' && validateFederatedUsername(user));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hasFederatedMembers = roomUsers.some((user: unknown) => typeof user === 'string' && user.includes(':') && user.includes('@')); | |
| const hasFederatedMembers = roomUsers.some((user: unknown) => typeof user === 'string' && validateFederatedUsername(user)); |
🤖 Prompt for AI Agents
In apps/meteor/ee/server/hooks/federation/index.ts around line 254, the current
predicate uses a loose string check (typeof user === 'string' &&
user.includes(':') && user.includes('@')) which is less strict than the
project's validateFederatedUsername; change the check to use
validateFederatedUsername by first ensuring user is a string and then calling
validateFederatedUsername(user as string) so only valid federated usernames are
counted; keep the type guard to avoid runtime errors and rely on the existing
imported validator for correctness.
| // when we receive extraData.federated, we need to prepare the room to be considered IRoomNativeFederated. | ||
| // according to isRoomNativeFederated for a room to be considered IRoomNativeFederated it is enough to have | ||
| // only an empty "federation" object | ||
| (extraData as IRoomNativeFederated).federation = { version: 1 } as any; |
There was a problem hiding this comment.
Type cast bypasses safety for incomplete federation object.
The code creates a federation object with only version, but IRoomNativeFederated.federation requires mrid and origin. The as any cast hides this mismatch from TypeScript. While the type guard isRoomNativeFederated only checks that federation exists, downstream code may attempt to access room.federation.mrid or room.federation.origin without null checks, leading to runtime errors.
Consider one of these approaches:
- Use a partial type to document the incomplete state:
- (extraData as IRoomNativeFederated).federation = { version: 1 } as any;
+ extraData.federation = { version: 1 } as Partial<IRoomNativeFederated['federation']> & { version: number };- Add a comment and ensure all access sites handle missing mrid/origin:
(extraData as IRoomNativeFederated).federation = { version: 1 } as any;
+ // NOTE: mrid and origin are set later by createRoom or setAsFederated.
+ // All code accessing federation.mrid or federation.origin must handle undefined.Then verify all access sites with:
#!/bin/bash
# Find all accesses to federation.mrid and federation.origin to ensure they handle undefined
ast-grep --pattern $'$_.federation.mrid'
ast-grep --pattern $'$_.federation.origin'🤖 Prompt for AI Agents
In apps/meteor/ee/server/hooks/federation/index.ts around line 289, the code
force-casts a federation object to any while only supplying version, hiding that
IRoomNativeFederated.federation requires mrid and origin; replace the unsafe
cast by assigning a properly typed partial (e.g. use
Partial<IRoomNativeFederated['federation']> or a dedicated type like { version:
number; mrid?: string; origin?: string }), or provide safe defaults for
mrid/origin if appropriate, and update the containing type if needed; then audit
all downstream accesses to federation.mrid and federation.origin to ensure they
handle undefined (add null checks or guards) and run the suggested ast-grep
search to verify no unsafe accesses remain.
Task: FB-44
Related PR: RocketChat/homeserver#300
Installing local homeserver version in Rocket.Chat
Run homeserver build and bundle:
To test this locally, from the Rocket.Chat project root run:
Make sure the homeserver repository is cloned at the same directory level as the Rocket.Chat repository.
How to test without the UI
Run this in the browser console while your local Rocket.Chat instance is running.
Test cases
INVITEDstatusINVITED; send system message thatrcUser2was invitedINVITEDINVITEDstatusINVITEDINVITED; RC sends system message “remote user invited”INVITED; RC sends system message “remote user invited”INVITED; send system message “user joined”INVITED; send system message “user joined”INVITED; send system message “user joined”INVITED; send system message “user joined”INVITED; send system message “user joined”INVITED; sends system messages when invited / accepted / rejected*while a subscription is in
INVITEDstatus, the RC user cannot perform any room actions except accept or reject the invite.*to all RC started invites, we should test in three different scenarios: using new channel creation modal, room members add user sidepanel and slash command.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.